Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Update Dockerfile to add support to arbitrary user ids #984

Merged
merged 3 commits into from
Dec 2, 2024

Conversation

edsoncsouza
Copy link
Contributor

About this change - What it does

This change updates the Dockerfile to handle dynamic User ID (UID) allocation in compliance with OpenShift best practices. It modifies file and directory permissions to ensure compatibility with OpenShift's security requirements, allowing containers to run with arbitrary UIDs while maintaining the required access to essential directories.

References: #983

Why this way

To solve the issue of dynamic UID handling, the proposed approach involves modifying permissions so that the container user can run with a dynamically assigned UID, which OpenShift uses for security purposes. Specifically, we change directory permissions and add the 'karapace' user to group '0', ensuring necessary access without granting root privileges. This adheres to OpenShift's recommendations for secure, multi-tenant container environments, allowing seamless and secure container operation.

@edsoncsouza edsoncsouza requested review from a team as code owners October 29, 2024 12:56
Copy link
Contributor

@jjaakola-aiven jjaakola-aiven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer the formatting related changes in separate commit and having the actual change in one.

container/Dockerfile Outdated Show resolved Hide resolved
container/Dockerfile Outdated Show resolved Hide resolved
container/Dockerfile Outdated Show resolved Hide resolved
container/Dockerfile Outdated Show resolved Hide resolved
container/Dockerfile Outdated Show resolved Hide resolved
Copy link
Contributor

@eliax1996 eliax1996 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except the nits.
Can we add a comment with the doc reference in the docker file?
I think reading this can be useful to understand why.

container/Dockerfile Outdated Show resolved Hide resolved
@edsoncsouza
Copy link
Contributor Author

I've made the corrections
Thanks for the reviews

@jjaakola-aiven jjaakola-aiven merged commit 2e8e717 into Aiven-Open:main Dec 2, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants